Open
Bug 712497
Opened 13 years ago
Updated 3 years ago
Clang Static Analysis: Dereference of undefined pointer value in nsprpub/pr/src/misc/prtpool.c
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
NEW
People
(Reporter: decoder, Unassigned)
References
(Blocks 1 open bug, )
Details
The following report (in the URL field) has been generated by static analysis using Clang.
It would be good if someone familiar with the particular code could check if
- this is really a bug or a false positive
- and/or if it makes sense to adjust the code (even if there is not a real bug present, e.g. by adding a missing initialization).
In this particular report, I think the problem is that | pollfds[pollfds_used] | could be accessed without taking the | if(..) | branch before that allocates this array. Even if this could never happen (which I don't know), it would probably be a good idea to initialize the | pollfds | pointer to NULL, so it would at most be a null pointer deference.
Updated•13 years ago
|
Assignee: nobody → wtc
Component: General → NSPR
Product: Core → NSPR
QA Contact: general → nspr
Version: Trunk → 4.9
Comment 1•13 years ago
|
||
Thanks for the bug report. This is a false positive.
The PRThreadPool structure pointed to by 'tp' is allocated by alloc_threadpool().
The structure is allocated with a PR_CALLOC call, so every member is initialized
to 0, including tp->ioq.cnt and tp->ioq.npollfds.
So the first time we enter the while loop in io_wstart, the code
pollfd_cnt = tp->ioq.cnt + 10;
if (pollfd_cnt > tp->ioq.npollfds) {
evaluates to true, and so we will do
pollfds = tp->ioq.pollfds;
Since this requires looking at three functions (alloc_threadpool,
PR_CreateThread, and io_wstart), it may be beyond the ability of
static analyzers.
If we initialize pollfds to NULL, will that fix this Clang report?
PRPollDesc *pollfds = NULL;
Reporter | ||
Comment 2•13 years ago
|
||
(In reply to Wan-Teh Chang from comment #1)
> Thanks for the bug report. This is a false positive.
>
Thanks for investigating!
> If we initialize pollfds to NULL, will that fix this Clang report?
>
> PRPollDesc *pollfds = NULL;
It would turn the message from "Dereference of undefined pointer" to "Possible null dereference". That would still help us though because we usually ignore the null dereferences (because they're not security critical in our context) and we can also annotate the code for that (I think, haven't looked into that yet).
So yes, changing that seems helpful although Clang will throw a different message then.
Comment 3•3 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: wtc → nobody
Updated•3 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•